Skip to content

[#219] Add rate limiting to view count API#224

Merged
realproject7 merged 2 commits intomainfrom
task/219-view-rate-limit
Mar 16, 2026
Merged

[#219] Add rate limiting to view count API#224
realproject7 merged 2 commits intomainfrom
task/219-view-rate-limit

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • In-memory rate limiter on POST /api/views: max 10 requests per IP per storyline per hour
  • Returns 429 Too Many Requests with Retry-After: 3600 header on excess
  • Periodic pruning of expired entries to prevent memory leak
  • IP extracted from x-forwarded-for header (standard for proxied deployments)

Test plan

  • Normal view tracking still works (< 10 per hour)
  • 11th request for same IP + storyline returns 429
  • After 1 hour, rate limit resets
  • Different storylines have independent limits

Fixes #219

🤖 Generated with Claude Code

In-memory rate limiter: max 10 POST /api/views requests per IP per
storyline per hour. Returns 429 with Retry-After header on excess.
Includes periodic pruning of expired entries to prevent memory leak.

Fixes #219

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b: APPROVE

Clean in-memory rate limiter:

  • Key: ip:storylineId — correctly scoped per IP per storyline
  • 10 req/hr window with periodic pruning to prevent memory leak
  • Rate limit check before any DB calls — protects Supabase
  • 429 with Retry-After header
  • IP from x-forwarded-for (always present on Vercel)

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The rate limiter is keyed too broadly for how view tracking currently works. On story pages we already emit one POST for the storyline plus one POST per rendered plot, so limiting only by IP + storylineId blocks legitimate plot-tracking traffic on longer stories.

Findings

  • [high] Story pages with many plots will hit the limit on first load
    • File: src/app/api/views/route.ts:13
    • File: src/app/api/views/route.ts:14
    • Suggestion: include plotIndex in the rate-limit key, or otherwise scope the limit to the actual page being viewed rather than the whole storyline. Right now the key is ${ip}:${storylineId}, but ViewTracker posts once for the storyline and once per PlotEntry, so a story with 10+ plots will start returning 429s to legitimate first-page loads even without abuse.

Decision

Requesting changes because the current limiter breaks the plot-level tracking introduced in #209 and violates the issue requirement that legitimate users remain unaffected.

Include plotIndex in rate limit key so a story with 10+ plots
doesn't exhaust the limit on first load. Each page surface
(storyline vs individual plot) now has independent limits.
Addresses T2a review feedback on PR #224.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The follow-up change fixes the limiter scope by including plotIndex in the rate-limit key, so storyline-level and per-plot tracking no longer contend for the same hourly bucket. The updated head now preserves legitimate first-load behavior while still rejecting excessive repeated POSTs.

Findings

  • None.

Decision

Approving because the previously blocking interaction with plot-level tracking is resolved, and GitHub lint-and-typecheck passes on the updated head.

@realproject7 realproject7 merged commit 10c154c into main Mar 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] Add rate limiting to view count API

2 participants